flags-stamp suite: an absent cc65 must be loud, not green (#177) - #186
Merged
JC-000 merged 2 commits intoSep 6, 2026
Merged
Conversation
JC-000
force-pushed
the
fix/177-flags-stamp-skip-loud
branch
from
September 5, 2026 23:28
b8db5a8 to
3a2024a
Compare
JC-000
added a commit
that referenced
this pull request
Sep 5, 2026
#180 (#178a) amended the module after this branch copied it, so the two copies are no longer identical (theirs c359a9b1..., the vendored one befcf309...) and `git merge-tree` reports an add/add conflict with five hunks in both directions. The "merges cleanly in either order" claim this branch shipped with is now false, and the PR body is corrected to match. Dropping the copy is free: this branch uses only require(), cannot_run() and EXIT_CANNOT_RUN, whose contracts did not change, and all five probes in tools/test_flags_stamp_skip_is_loud.py behave identically against either version of the module. Merge order is therefore load-bearing and stated in the PR body: #183 (this branch's base) -> #180 (brings _skip_policy.py) -> #186. Also migrates the guard in test_an_unrelated_option_does_not_suppress_invalidation, added to the base branch after this one was written, to _require_toolchain(). It was the one remaining site still spelling the silent-skip prologue by hand, which is the whole point of #177. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
tools/test_build_flags_stamp.py has eight tests, every one of which shells
out to `make`. Each opened with
missing = _toolchain_missing()
if missing:
print(f"SKIP: {missing} not on PATH")
return
and a pytest test function that returns None without asserting is a PASS.
The module is pinned in pytest.ini `testpaths`, so on a machine without
cc65 -- a CI container, most likely -- bare `pytest` at the repo root
reported `7 passed in 0.01s` having verified nothing. The SKIP lines were
not even visible: pytest captures stdout on a passing test and discards it.
The standalone runner had the same hole from the other side, printing
`PASSED: 0 failure(s)` for a run in which zero assertions executed.
The invariant being laundered is build/flags.stamp itself -- what closes
CLAUDE.md's two documented silent-failure modes. #158 adopted the rule (an
involuntary skip is a failure; a voluntary skip is allowed but must never be
silent), #157 reintroduced it, PR #172 closed it again.
Both channels now:
pytest lane _require_toolchain() calls _skip_policy.require(), which
raises SkipPolicyError carrying the vacuity warning
INSIDE the reason string -- under `-ra` the reason is the
only channel that survives.
standalone lane main() hoists the check and returns cannot_run() = exit
2, distinct from 1 ("a check ran and failed"), plus a
ran+failed==0 guard so PASSED can never be printed by a
run that executed nothing.
opt-out C64_ALLOW_SKIP=1 buys exit 0 / a real pytest skip and
still prints the warning; exactly "1", so
C64_ALLOW_SKIP=0 does NOT open the hatch.
tools/_skip_policy.py is vendored from the #178 Part A work, byte-identical
to the copy on branch test/178a-skip-policy-rig-half (c3cf8a4), so this PR
is green standing alone rather than importing a module that is not on master
yet. An identical both-added file merges without conflict in either order.
That branch also adds tools/test_skip_policy.py (unit tests for the module)
and wires the rig lane; none of that is duplicated here.
New tools/test_flags_stamp_skip_is_loud.py is the red-green. It re-runs
the subject module in a subprocess whose PATH has had every directory
containing ca65/ld65 removed -- computed from the real PATH, not a
hardcoded /usr/bin:/bin, so it strips the toolchain wherever it is
installed. test_the_strip_actually_strips guards the guard: a
PATH-stripping bug would otherwise make every assertion vacuous, one level
up from the defect being fixed.
Measured, with the subject module at the pre-fix revision:
FAIL test_opt_out_is_explicit_and_still_warns
FAIL test_pytest_run_without_toolchain_is_not_green
FAIL test_standalone_run_without_toolchain_exits_cannot_run
ok test_the_strip_actually_strips
FAILED: 3 failure(s) (4 executed)
and after:
PASSED: 0 failure(s) (4 executed)
Bare `pytest` at the repo root: 60 passed (was 56; +4 from the new module).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#180 (#178a) amended the module after this branch copied it, so the two copies are no longer identical (theirs c359a9b1..., the vendored one befcf309...) and `git merge-tree` reports an add/add conflict with five hunks in both directions. The "merges cleanly in either order" claim this branch shipped with is now false, and the PR body is corrected to match. Dropping the copy is free: this branch uses only require(), cannot_run() and EXIT_CANNOT_RUN, whose contracts did not change, and all five probes in tools/test_flags_stamp_skip_is_loud.py behave identically against either version of the module. Merge order is therefore load-bearing and stated in the PR body: #183 (this branch's base) -> #180 (brings _skip_policy.py) -> #186. Also migrates the guard in test_an_unrelated_option_does_not_suppress_invalidation, added to the base branch after this one was written, to _require_toolchain(). It was the one remaining site still spelling the silent-skip prologue by hand, which is the whole point of #177. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JC-000
force-pushed
the
fix/177-flags-stamp-skip-loud
branch
from
September 5, 2026 23:41
3a2024a to
3619583
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #177. The eighth site in #178's survey.
Merge order is load-bearing
This branch imports
tools/_skip_policy.pyand no longer carries a copy.An earlier revision vendored it, on the reasoning that an identical both-added
file merges cleanly in either order. #180 then amended the module and
git merge-treereported an add/add conflict in both directions, so that claimis retracted and the copy is dropped instead. No sha is quoted here on
purpose: the module has moved three times while this PR was open, which is
itself the argument for importing rather than vendoring. The authority is
origin/test/178a-skip-policy-rig-half, whatever it currently points at.Dropping the copy costs nothing — this branch uses only
require(),cannot_run()andEXIT_CANNOT_RUN, whose contracts have not changed throughany of those revisions.
Verified against that branch's current head: the merge is conflict-free
(including
pytest.ini, which ends up with all three newtestpathslines),the merged tree's module is byte-identical to the one #180 provides — so it
comes from exactly one place — and on that tree
python3 tools/test_flags_stamp_skip_is_loud.pyisPASSED: 0 failure(s) (4 executed)with barepytestat 92 passed. Until #180 lands, thisbranch alone is
1 errorand cannot import; every measurement below was takenon the merged tree.
The defect
Eight tests, each shelling out to
make, each opening withA pytest test that returns
Nonewithout asserting is a pass, and themodule is pinned in
pytest.initestpaths— so on a machine without cc65bare
pytestat the repo root goes green while asserting thebuild/flags.stampinvariant it never checked. TheSKIP:lines are not evenvisible: pytest captures stdout on a passing test and discards it.
Red / green
New
tools/test_flags_stamp_skip_is_loud.pyre-runs the subject module in asubprocess whose
PATHhas had every directory containingca65/ld65removed — computed from the real
PATH, not a hardcoded/usr/bin:/bin,so it strips the toolchain wherever it happens to be installed.
RED, subject module at the pre-fix revision:
with the subject's own output captured in the failure message — the exact shape
#177 reports (
SKIP: ca65 not on PATH/ok× 8, thenPASSED: 0 failure(s)).GREEN, after the fix:
PASSED: 0 failure(s) (4 executed), and identicallyagainst #180's amended module.
Both channels after the fix, toolchain stripped:
With cc65 present nothing changes: bare
pytestat the repo root is92 passed on the merged tree described above.
Shape of the fix
return→ passrequire()raisesSkipPolicyError; the reason string carries the vacuity warning, because under-rathe reason is the only surviving channelPASSED: 0 failure(s), exit 0cannot_run()→ exit 2, distinct from 1 ("a check ran and failed"), plus aran + failed == 0guard soPASSEDcan never be printed by a run that executed nothingC64_ALLOW_SKIP=1buys exit 0 / a real pytest skip and still prints the warning; exactly"1", soC64_ALLOW_SKIP=0does not open the hatch (asserted)main()hoists the toolchain check out of the loop and callscannot_run()directly, rather than letting eight
require()calls report eight identicalfailures for one missing binary. Review confirmed this is also why the
standalone lane's behaviour is unaffected by the
_skip_policy.pyrevision:it never reaches
require().test_the_strip_actually_stripsguards the guard — aPATH-stripping bugwould make every other assertion run against a machine that can build, and
they would all pass having tested nothing: the same vacuity, one level up.
Known follow-up, not a blocker
_toolchain_missing()checks onlyca65/ld65. An uninitialised submodule(no
libs/nistcurvesworking tree) is a different failure:farm.make()asserts and the suite reports eight loud failures rather than a named
COULD NOT RUN. Loud is the correct half; the naming is not. Worth a separatepass, and it does not affect what this PR pins — #177's own repro is verbatim a
PATHstrip producing seven silent passes.🤖 Generated with Claude Code